-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: don't modify manifest on failing change #2756
fix: don't modify manifest on failing change #2756
Conversation
Why are the intermediate saves needed? This implementation seems quite
hacky.
…On Fri, 20 Dec 2024 at 17:17, Ruben Arts ***@***.***> wrote:
Fixes: #2516 <#2516>
We still need to figure out if we could get rid of the intermediary saves
but this will help the users with reverting the changes if it fails.
------------------------------
You can view, comment on, or merge this pull request online at:
#2756
Commit Summary
- bf64873
<bf64873>
fix: undo changes to manifest if it fails to solve and install on add or
upgrade
File Changes
(4 files <https://github.com/prefix-dev/pixi/pull/2756/files>)
- *M* src/cli/add.rs
<https://github.com/prefix-dev/pixi/pull/2756/files#diff-52e88acc3ab9bc36d40fb6a637c96b8b82a20b9b9d8e5f64adb5990952773c84>
(17)
- *M* src/cli/upgrade.rs
<https://github.com/prefix-dev/pixi/pull/2756/files#diff-63422bd7313d222b0647e871e2379a5e0225f0eeb308e8964a934823f2273d46>
(16)
- *M* src/project/mod.rs
<https://github.com/prefix-dev/pixi/pull/2756/files#diff-78c425ecb327c26438b6d71f1aa594b30d4903afa5ddc55ada2dbc31c8ec9d22>
(4)
- *M* tests/integration_python/test_main_cli.py
<https://github.com/prefix-dev/pixi/pull/2756/files#diff-172c65bd6d2ae80bc65af36f45fc42cf58d350955039462164406e72228e417a>
(18)
Patch Links:
- https://github.com/prefix-dev/pixi/pull/2756.patch
- https://github.com/prefix-dev/pixi/pull/2756.diff
—
Reply to this email directly, view it on GitHub
<#2756>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGDW733IO32KNKR5I7YRFT2GQ7KBAVCNFSM6AAAAABT7NTF7WVHI2DSMVQWIX3LMV43ASLTON2WKOZSG42TGMBUGM4TAMQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
@baszalmstra Because we need to update the dependencies directly in file to allow for correct solving of the PyPI environment when it is used as a |
This also doesn't work with interrupting the |
The context is that we need to save the If we save, we also need to save in order to revert the change if things go wrong. |
Can we only do it for that specific case then? |
can we pass pyproject.toml from a tmp location to uv? or maybe uv can read it from stdin? this case we don't need to save it back on disk to the original location |
@nichmor that would not work for relative path dependencies? |
could we change it to absolute during resolving/building? |
@nichmor I'm kinda unsure how this solves anyhing, as we are not letting uv read the |
I changed the logic so that we only intermediary save when the manifest is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get this in :)
Fixes: #2516
We still need to figure out if we could get rid of the intermediary saves but this will help the users with reverting the changes if it fails.